Add waiting state to showcase form #1605
Conversation
✅ Deploy Preview for codingtrain ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Can we use the pseudo class :disabled instead of the attribute disabled? We might not need all the color variants specificity here either.
.rainbow:hover:not(:disabled) {
background-position: left bottom;
transition: background-position 4s ease-out;
}
button:disabled {
opacity: 0.5;
cursor: not-allowed;
}And well done on the negation here, I was wondering why you didn't use :enabled initially but this button component is sneaky and can end up being a <button> or a <Link> / <a> tag based on the props passed to it.
There was a problem hiding this comment.
Thought about this a bit more, and maybe the :not(:disabled) bit is too "clever" without a comment? Maybe something like this would be clearer/more self-documenting in the long run.
a.rainbow, button.rainbow:enabled {
&:hover {
background-position: left bottom;
transition: background-position 4s ease-out;
}
}| @@ -64,6 +64,6 @@ | |||
| } | |||
| } | |||
|
|
|||
| .submitBtn:hover { | |||
| .submitBtn:hover:not([disabled]) { | |||
There was a problem hiding this comment.
Here we could actually use :enabled since it is always a button
| @@ -429,7 +435,7 @@ const PassengerShowcaseForm = () => { | |||
| className={css.submitBtn} | |||
| onClick={onSubmit} | |||
| variant="purple" | |||
| disabled={submitted} | |||
| disabled={submitted !== 'not-submitted'} | |||
There was a problem hiding this comment.
Nit-pick but I find it hard to parse cognitively with the double (triple?) negative.
Maybe renaming the variable to submissionState and finding an alternate name for not-submitted state that doesn't contain a negative would help? What about undefined, would that read better?
submissionState === 'submitted'
disabled = !!submissionState
|
Looks great, I agree! Sorry for not shipping a fix myself after self-assigning the issue. I've been taking notes on additional improvements to the submission process and did not get a chance to code or share anything yet. More soon-ish! |
|
Thank you @dipamsen, excited to merge this! |
|
Oh whoops looks like the review comments were pending to be addressed... |
|
Oh, sorry about that! Would you like me to revert or shall we just start a new branch? Apologies for this mistake! |
|
No worries, and no need to revert since the comments were more about minor code quality improvements. I can address them in an upcoming PR. |
See #1550
disabledbutton properlycc @fturmel